Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[META 319] Align sanitize_field_names with cross-agent spec #982

Merged
merged 10 commits into from
Jan 28, 2021

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Dec 3, 2020

What does this pull request do?

Aligns the python agent's behavior and default for the sanitize_field_names config with that defined in the cross-agent spec

Note that this is a breaking change. Previously, entries in sanitize_field_names would be checked with an if pattern in key check. This meant that "secret" would match (and mask) "the_secret". With the change to wildcard glob matching, this will no longer be the case. Users should add stars to each side of their matches (i.e. "*secret*") in order to retain the previous behavior.

The value sanitization of credit card numbers has also been removed. The credit card validation was of dubious value and removing it will improve performance.

Additionally, the sanitization of request querystrings has been removed.

Finally, the sanitization mask has been changed to "[REDACTED]", to match the recommendation from elastic/apm#378.

This PR should be merged for version 6.0, which will coincide with the 7.11 stack release.

Related issues

Closes #904
Closes #920

@basepi basepi modified the milestones: 7.11, 6.0 Dec 3, 2020
@apmmachine
Copy link
Contributor

apmmachine commented Dec 3, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #982 updated

    • Start Time: 2021-01-28T18:57:38.685+0000
  • Duration: 23 min 4 sec

  • Commit: 690f70c

Test stats 🧪

Test Results
Failed 0
Passed 8372
Skipped 5888
Total 14260

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 8372
Skipped 5888
Total 14260

@basepi basepi marked this pull request as ready for review December 3, 2020 19:25
@basepi basepi requested a review from beniwohli December 3, 2020 19:25
Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one nitpick, other than that LGTM!

elasticapm/conf/constants.py Show resolved Hide resolved
@basepi basepi merged commit 1527843 into elastic:master Jan 28, 2021
beniwohli pushed a commit to beniwohli/apm-agent-python that referenced this pull request Sep 14, 2021
)

* Align sanitize_field_names with cross-agent spec

* Add CHANGELOG

* Remove value sanitization

The value of the credit card check was dubious, and this change will
improve performance.

* Move to MASK recommended by elastic/apm#378

* Update changelog

* Remove querystring sanitization

* Update pre-commit to match upstream black behavior

* Dynamically generate BASE_SANITIZE_FIELD_NAMES

* Fix the merge commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove query string sanitization in 6.0 [META 319] Add sanitize_field_names to remote config
3 participants